Jsonp tck #232
Jsonp tck #232
Conversation
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
|
@aguibert I cannot add you as reviewer but it makes sense that you take a look on this if you want. |
| <version>1.0.0-SNAPSHOT</version> | ||
| </parent> | ||
|
|
||
| <artifactId>jakarta.json-tck-tests-plugability</artifactId> |
There was a problem hiding this comment.
Why are there 3 poms in the impl-tck folder? It's not clear how "plugability" tests are different from the other ones
There was a problem hiding this comment.
See it in my comment below. In summary some tests are running with a custom json provider and others don't.
| <module>tck-common</module> | ||
| <module>tck-tests</module> | ||
| <module>tck-tests-plugability</module> |
There was a problem hiding this comment.
Again, not sure why there are 3 separate modules here. Seems like there could just be one?
There was a problem hiding this comment.
In current jakartaee-tck, when you execute the jsonp tests it generates a separate jar named "jsonp_alternate_provider.jar". It is done in that way because
Plugability is running the tests with a custom json provider in /META-INF/services.
|
|
||
| import junit.framework.AssertionFailedError; | ||
|
|
||
| public class Fault extends AssertionFailedError { |
There was a problem hiding this comment.
This exception class isn't really needed. Can just use the standard junit exceptions
There was a problem hiding this comment.
I did it in that way to dramatically decrease the modifications in the CTS tests. There are many methods throwing that Fault.
We need to live for some time with this copy-pasted tests, so if a modification happens in jakartaee-tck, we need to update this tests too. For the human eye it will be easier to see differences.
|
Thanks for looking into this @jbescos! I added some comments mainly around the project structure. However, I think we should try to nail down a strategy on the mailing list before other specs try to copy the proposal I did for JSON-B. My ultimate goal is to move the TCKs from the monorepo to the spec repos (as opposed to duplicating them), but we don't yet have agreement on that. |
|
@aguibert thanks for reviewing it. Definitely this simplifies a lot the execution of CTS tests for different implementations. I'm afraid it is not doable for all cases, because some tests requires a running web application server. |
Similarly as it is done in Jsonb as API and Yasson as implementation, I have added CTS tests in Jsonp.
The idea is that any Jsonp implementation (including jsonp/impl) can easily execute CTS tests.
TCK folder:
It contains the tests that already exists under: https://github.com/eclipse-ee4j/jakartaee-tck/tree/master/src/com/sun/ts/tests/jsonp
I had to do some modifications in the tests, but not in the logic:
Sigtests are not included yet. I checked that for jsonb is not in place either. I need to think about re-usability of common classes that other projects (jpa, ejb, jsonp, jsonb, etc) requires. Or we can always copy paste them everywhere.
Impl-TCK folder:
Here it run the current implementation to CTS tests.
Travis will run the CTS tests automatically so we can find issues without releasing a new Glassfish.